Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fix](Variant) fix variant with not null #32248

Merged
merged 2 commits into from
Mar 19, 2024

Conversation

eldenmoon
Copy link
Member

ignore null bitmap for not null and make subcolumn access slots always nullable

Proposed changes

Issue Number: close #xxx

Further comments

If this is a relatively large or complex change, kick off the discussion at [email protected] by explaining why you chose the solution you did and what alternatives you considered, etc...

@doris-robot
Copy link

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR

ignore null bitmap for not null and make subcolumn access slots always nullable
@eldenmoon
Copy link
Member Author

run buildall

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 34.99% (8573/24498)
Line Coverage: 26.74% (69455/259770)
Region Coverage: 26.02% (36078/138675)
Branch Coverage: 22.97% (18422/80200)
Coverage Report: http://coverage.selectdb-in.cc/coverage/f1f5f1d03f6b25519369a6ecb011b5936deb24da_f1f5f1d03f6b25519369a6ecb011b5936deb24da/report/index.html

@doris-robot
Copy link

TPC-H: Total hot run time: 38275 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit f1f5f1d03f6b25519369a6ecb011b5936deb24da, data reload: false

------ Round 1 ----------------------------------
q1	17658	4222	4073	4073
q2	2016	156	144	144
q3	10594	1109	880	880
q4	7431	747	702	702
q5	7454	2768	2765	2765
q6	182	122	123	122
q7	1165	827	797	797
q8	9320	2045	1979	1979
q9	7184	6418	6364	6364
q10	8516	3567	3621	3567
q11	432	227	214	214
q12	704	308	289	289
q13	18058	2882	2861	2861
q14	274	244	258	244
q15	496	453	452	452
q16	514	395	395	395
q17	953	546	549	546
q18	7141	6456	6427	6427
q19	1589	1445	1404	1404
q20	548	281	266	266
q21	6159	3487	3620	3487
q22	345	297	318	297
Total cold run time: 108733 ms
Total hot run time: 38275 ms

----- Round 2, with runtime_filter_mode=off -----
q1	4079	4050	4010	4010
q2	318	216	220	216
q3	2958	2848	2809	2809
q4	1817	1599	1575	1575
q5	5182	5231	5209	5209
q6	194	121	117	117
q7	2232	1865	1858	1858
q8	3185	3257	3249	3249
q9	8539	8551	8524	8524
q10	3637	3685	3670	3670
q11	553	442	434	434
q12	707	529	542	529
q13	16926	2826	2876	2826
q14	291	251	253	251
q15	488	452	447	447
q16	455	420	434	420
q17	1719	1486	1487	1486
q18	7435	7228	7121	7121
q19	1618	1518	1505	1505
q20	1901	1689	1706	1689
q21	4798	4713	4602	4602
q22	532	462	447	447
Total cold run time: 69564 ms
Total hot run time: 52994 ms

slotDescriptor.setMaterializedColumnName(slotRef.getColumnName()
+ "." + String.join(".", slotReference.getSubColPath()));
// always nullable at present
slotDescriptor.setIsNullable(true);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if variant slot always nullable, u should set it correctly in Nereids, because nereids need nullable info todo some optimization. if u set uncorrect nullable info, we will get wrong plan sometimes

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@eldenmoon
Copy link
Member Author

run buildall

@eldenmoon eldenmoon requested a review from morrySnow March 15, 2024 05:15
@doris-robot
Copy link

TPC-H: Total hot run time: 38316 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 7cf5bb4bc67187fea7bb803d41139bc8b47f9a75, data reload: false

------ Round 1 ----------------------------------
q1	17656	4295	4162	4162
q2	2034	164	146	146
q3	10575	1102	878	878
q4	7779	714	759	714
q5	7460	2758	2738	2738
q6	187	125	133	125
q7	1168	813	800	800
q8	9436	2015	2002	2002
q9	7268	6414	6438	6414
q10	8525	3513	3648	3513
q11	421	223	224	223
q12	780	305	281	281
q13	18125	2848	2825	2825
q14	276	241	252	241
q15	501	451	441	441
q16	508	389	390	389
q17	948	550	539	539
q18	7191	6457	6361	6361
q19	1909	1502	1410	1410
q20	534	289	275	275
q21	6032	3539	3579	3539
q22	349	316	300	300
Total cold run time: 109662 ms
Total hot run time: 38316 ms

----- Round 2, with runtime_filter_mode=off -----
q1	4174	4176	4123	4123
q2	324	225	219	219
q3	2927	2831	2836	2831
q4	1838	1527	1483	1483
q5	5230	5206	5235	5206
q6	198	118	119	118
q7	2234	1881	1845	1845
q8	3163	3303	3273	3273
q9	8496	8540	8516	8516
q10	3689	3732	3660	3660
q11	537	450	433	433
q12	729	560	570	560
q13	16911	2848	2868	2848
q14	283	251	256	251
q15	502	446	452	446
q16	473	417	398	398
q17	1744	1498	1469	1469
q18	7496	7143	7069	7069
q19	1675	1513	1496	1496
q20	1891	1684	1691	1684
q21	4635	4807	4646	4646
q22	543	463	441	441
Total cold run time: 69692 ms
Total hot run time: 53015 ms

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 34.95% (8574/24534)
Line Coverage: 26.65% (69473/260722)
Region Coverage: 25.94% (36086/139120)
Branch Coverage: 22.89% (18423/80488)
Coverage Report: http://coverage.selectdb-in.cc/coverage/7cf5bb4bc67187fea7bb803d41139bc8b47f9a75_7cf5bb4bc67187fea7bb803d41139bc8b47f9a75/report/index.html

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Mar 18, 2024
Copy link
Contributor

PR approved by at least one committer and no changes requested.

Copy link
Contributor

PR approved by anyone and no changes requested.

@eldenmoon
Copy link
Member Author

run p0

Copy link
Contributor

@xiaokang xiaokang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@eldenmoon eldenmoon merged commit 3d4af5c into apache:master Mar 19, 2024
26 of 31 checks passed
@eldenmoon eldenmoon deleted the fix-var-not-null branch March 19, 2024 03:16
yiguolei pushed a commit that referenced this pull request Mar 21, 2024
ignore null bitmap for not null and make subcolumn access slots always nullable
yiguolei pushed a commit that referenced this pull request Mar 21, 2024
ignore null bitmap for not null and make subcolumn access slots always nullable
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by one committer. dev/2.1.0 dev/2.1.1 reviewed variant
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants